Skip to content

Do not use core Airflow Flask related resources in FAB provider#45441

Merged
vincbeck merged 2 commits into
apache:mainfrom
aws-mwaa:vincbeck/fab_extensions
Jan 7, 2025
Merged

Do not use core Airflow Flask related resources in FAB provider#45441
vincbeck merged 2 commits into
apache:mainfrom
aws-mwaa:vincbeck/fab_extensions

Conversation

@vincbeck

@vincbeck vincbeck commented Jan 6, 2025

Copy link
Copy Markdown
Contributor

As part of AIP-79.

The Flask application in FAB provider should not use any Flask related resources from core Airflow. Otherwise, deleting the main Flask application in core Airflow when the legacy Airflow 2 is gone will be impossible.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

auth.init_app(app)
app.api_auth.append(auth)
except ImportError as err:
log.critical("Cannot import %s for API authentication due to: %s", backend, err)

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information

This expression logs [sensitive data (secret)](1) as clear text. This expression logs [sensitive data (secret)](2) as clear text.
Comment thread airflow/auth/managers/base_auth_manager.py
Comment thread providers/src/airflow/providers/fab/www/extensions/init_session.py
Comment thread providers/src/airflow/providers/fab/www/security_manager.py
Comment thread providers/src/airflow/providers/fab/www/session.py
Comment thread providers/src/airflow/providers/fab/www/utils.py
Comment thread providers/src/airflow/providers/fab/www/views.py
@vincbeck
vincbeck force-pushed the vincbeck/fab_extensions branch 5 times, most recently from 7eb4744 to 3294c71 Compare January 6, 2025 22:21
@vincbeck
vincbeck force-pushed the vincbeck/fab_extensions branch from 3294c71 to f3e31ca Compare January 6, 2025 23:13

if TYPE_CHECKING:
from airflow.www.extensions.init_appbuilder import AirflowAppBuilder
from airflow.providers.fab.www.extensions.init_appbuilder import AirflowAppBuilder

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vincbeck We don't have add_permissions() method in from airflow.providers.fab.www.extensions.init_appbuilder import AirflowAppBuilder class. Causing airflow sync-perm command to fail on the main branch.

Updating actions and resources for all existing roles
webserver  | Traceback (most recent call last):
webserver  |   File "/usr/local/bin/airflow", line 8, in <module>
webserver  |     sys.exit(main())
webserver  |              ^^^^^^
webserver  |   File "/usr/local/lib/python3.12/site-packages/airflow/__main__.py", line 58, in main
webserver  |     args.func(args)
webserver  |   File "/usr/local/lib/python3.12/site-packages/airflow/cli/cli_config.py", line 49, in command
webserver  |     return func(*args, **kwargs)
webserver  |            ^^^^^^^^^^^^^^^^^^^^^
webserver  |   File "/usr/local/lib/python3.12/site-packages/airflow/utils/cli.py", line 111, in wrapper
webserver  |     return f(*args, **kwargs)
webserver  |            ^^^^^^^^^^^^^^^^^^
webserver  |   File "/usr/local/lib/python3.12/site-packages/airflow/utils/providers_configuration_loader.py", line 55, in wrapped_function
webserver  |     return func(*args, **kwargs)
webserver  |            ^^^^^^^^^^^^^^^^^^^^^
webserver  |   File "/usr/local/lib/python3.12/site-packages/airflow/providers/fab/auth_manager/cli_commands/sync_perm_command.py", line 35, in sync_perm
webserver  |     appbuilder.add_permissions(update_perms=True)
webserver  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
webserver  | AttributeError: 'AirflowAppBuilder' object has no attribute 'add_permissions'. Did you mean: '_add_permission'?

Is there an alternative method we should use?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it still happening? The solution would be to copy over add_permissions from core Airflow to FAB provider

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vincbeck, I have created a PR for the same. PTAL - #45611

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants